Skip to content

fix(server): register the plugin worker manager per process so run dispatch cannot lose it - #306

Open
stubbi wants to merge 1 commit into
mainfrom
fix/plugin-worker-process-registry
Open

fix(server): register the plugin worker manager per process so run dispatch cannot lose it#306
stubbi wants to merge 1 commit into
mainfrom
fix/plugin-worker-process-registry

Conversation

@stubbi

@stubbi stubbi commented Jul 26, 2026

Copy link
Copy Markdown

The bug, for the third time

Runs fail with:

Failed to acquire lease for environment "Kubernetes Sandbox" (sandbox): Sandbox provider "kubernetes" is installed, but sandbox plugin workers are unavailable in this server process.

Seen on production 2026-07-26: three runs for one company, dispatched by assignment and automation, failing in 0.37s to 0.42s. Sub-second is the diagnostic tell for a wiring gap; a genuinely absent worker polls pluginWorkerReadyTimeoutMs (5s) first.

Why patching call sites has not worked

The plugin worker manager is process-scoped, which is what the message itself says. But it is threaded as an optional parameter through six environmentRuntimeService construction sites and dozens of route mounts. Any caller that omits it builds a run engine that can never acquire a sandbox lease, and the omission is invisible until a customer hits that exact path.

#304 fixed three route mounts. It could not fix these, which pass no options at all:

  • server/src/routes/activity.ts:25
  • server/src/routes/instance-settings.ts:31
  • server/src/routes/summary-slots.ts:31
  • server/src/services/companies.ts:59

heartbeatService forwards options.pluginWorkerManager straight into environmentRuntimeService (heartbeat.ts:5594), so heartbeatService(db) yields a manager-less runtime. That is the path assignment and automation take.

The fix

Register the manager once at app construction, and have the sandbox driver fall back to it:

// createSandboxEnvironmentDriver
const pluginWorkerManager = options.pluginWorkerManager ?? getProcessPluginWorkerManager();

Every dispatch path funnels through createSandboxEnvironmentDriver, so one fallback covers all of them, including any construction site added in future.

Properties kept deliberately:

  • An explicitly passed manager still wins, so existing tests keep injecting their own and are unaffected.
  • No manager is still not a crash. Contexts that legitimately have none (migrations, CLI entrypoints, unit tests that never dispatch) get the same accurate, non-retryable error as before. isSandboxProviderWorkerUnavailableFailureMessage matches on message text, so retry classification is unchanged.

Alternative considered

Making pluginWorkerManager required in the types would be compile-time enforced, but it is a far larger change across every construction site, breaks contexts that genuinely have no worker, and is still bypassable with undefined as any.

Tests

New server/src/__tests__/plugin-worker-process-registry.test.ts, 5 cases: empty registry returns undefined, registration round-trips, re-registration replaces, an explicit undefined is treated as absent rather than throwing, and an explicitly passed manager takes precedence over the process registration.

…spatch cannot lose it

The plugin worker manager is process-scoped, which is exactly what the failure
message says: "sandbox plugin workers are unavailable in this server process".
But it was threaded as an OPTIONAL parameter through six runtime construction
sites and dozens of route mounts, so any caller that omitted it built a run
engine that could never acquire a sandbox lease, and the omission only surfaced
when a customer hit that specific path.

That has now bitten three times. #304 fixed three route mounts, but four
`heartbeatService(db)` calls pass no options at all (routes/activity.ts,
routes/instance-settings.ts, routes/summary-slots.ts, services/companies.ts),
and heartbeat forwards `options.pluginWorkerManager` straight into
`environmentRuntimeService`. Observed in production on 2026-07-26: three runs
for one company failed to acquire a lease, dispatched by `assignment` and
`automation`, each failing in 0.37s to 0.42s. Sub-second is the tell for a
wiring gap; a genuinely absent worker polls for 5s first.

Fix the class rather than the instances: register the manager once at app
construction and have the sandbox driver fall back to it. That single fallback
covers every dispatch path, because they all funnel through
createSandboxEnvironmentDriver. An explicitly passed manager still wins, so
tests keep injecting their own, and contexts that legitimately have no worker
(migrations, CLI) still get the accurate non-retryable error rather than a
crash.

Chose this over making the parameter required: that is a far larger change
across every construction site, and it is still bypassable with
`undefined as any`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant